Add DCSR and DPC CSRs #614

neverlandiz · 2025-04-14T18:02:08Z

This PR addresses Issue #570 and adds the remaining missing debug CSRs

DCSR
DPC

neverlandiz · 2025-04-14T18:03:20Z

For core debug registers like DCSR and DPC, it’s stated that “these registers are only accessible from Debug Mode” (Section 4.9, debug specs). What would be the correct priv_mode for those CSRs? Debug mode isn’t one of the options in the schema (only M, S, U, VS).

ThinkOpenly · 2025-04-23T15:29:12Z

For core debug registers like DCSR and DPC, it’s stated that “these registers are only accessible from Debug Mode” (Section 4.9, debug specs). What would be the correct priv_mode for those CSRs? Debug mode isn’t one of the options in the schema (only M, S, U, VS).

Hmm. The Priv spec 1.3 "Debug Mode" says:

Debug mode (D-mode) can be considered an additional privilege mode, with even more access than M-mode.

That sounds like a full Mode (capital M). It probably needs to be treated as such, adding a new mode in schemas/csr_schema.json, and an entry in arch/isa/globals.isa, but there don't appear to be any CSR bits to identify "D-mode" (I couldn't find any). So, which new enum value to choose is an open question. Maybe @dhower-qc has an idea?

Some additional information: arch/isa/globals.isa has:

# encoded as defined in the privilege spec
enum PrivilegeMode {
  M  0b011
  S  0b001
  HS 0b001 # alias for S when H extension is used
  U  0b000
  VS 0b101
  VU 0b100
}

These 3 bits appear to be the concatenation of MPV and MPP, or at least they line up that way in the Priv spec, 18.4.1 "Machine Status Registers (mstatus and mstatush)", table 32:

It may be that you could just pick a random new enum value (prepend a 4th bit, like 0b1000?), but I don't know for sure. (You could certainly try!)

dhower-qc · 2025-05-05T17:55:35Z

Yea, we do need to add D-mode in globals.isa, and yes, there are cases where we are relying on the encoding of PrivilegeMode being the table Paul added above. For example:

riscv-unified-db/arch/isa/globals.isa

Lines 628 to 634 in 00f1540

    
           $pc = {CSR[stvec].BASE, 2'b00}; 
        
           CSR[scause].INT = 1'b0; 
        
           CSR[scause].CODE = $bits(code); 
        
           CSR[hstatus].GVA = 1; 
        
           CSR[hstatus].SPV = 1; # guest page faults always come from a virtual mode 
        
           CSR[hstatus].SPVP = $bits(from_mode)[0]; 
        
           CSR[mstatus].SPP = $bits(from_mode)[0];

As such, we should encode D as 0b1011 since it is M (0b0011) with extra permission.

arch/csr/dcsr.yaml

ThinkOpenly · 2025-05-13T14:49:31Z

arch/csr/dpc.yaml

+long_name: Debug PC Register
+address: 0x7B1
+priv_mode: D
+length: XLEN


I think we need a "DXLEN" here. From The RISC-V Debug Specification:

DXLEN: Debug XLEN, which is the widest XLEN a hart supports, ignoring the current value of MXL in misa.

Also, needs to be added to schemas/csr_schema.json as a valid enum.

ThinkOpenly · 2025-05-13T14:52:22Z

arch/csr/dpc.yaml

+    location_rv32: 31-0
+    location_rv64: 63-0
+    type: RW
+    description: DPC Value


change "DPC" to "Debug PC"

arch/isa/globals.isa

…ntation

ThinkOpenly · 2025-05-21T15:47:52Z

arch/csr/dcsr.yaml

+      if (!MPRVEN_IMPLEMENTED) {
+        unimplemented_csr($encoding);
+      }


Add return of the value to be written:

return csr_value.MPRVEN;

Given this statement:

"Implementing this bit is optional. It may be tied to either 0 or 1."

I think the behavior is that the bit always "exists"/won't cause an exception. Rather, it can be:

tied to 0

tied to 1

R/W

So, I think the parameter needs to enumerate those possibilities, and based on that, we need a dynamic type()/reset_value(). Since sw_write() doesn't apply to Read-Only bits, and when it is writable there are no restrictions, we don't actually need sw_write().

ThinkOpenly · 2025-05-21T16:01:28Z

One of the regression tests is failing:

/home/runner/work/riscv-unified-db/riscv-unified-db/lib/arch_obj_models/csr.rb:215:in `length': Unexpected length field for dpc (RuntimeError)

Might need a hint from @dhower-qc here, but adding DXLEN support might also include:

Adding a value for it in cfgs/example_rv64_with_overlay.yaml
Updating arch/csr/schema.adoc
Updating arch/README.adoc
Maybe updating lib/arch_obj_models/csr.rb (This is the spot which might be causing the failure.)

dhower-qc · 2025-05-21T16:51:33Z

arch/csr/dcsr.yaml

+      if (!MPRVEN_IMPLEMENTED) {
+        unimplemented_csr($encoding);
+      }


Given this statement:

"Implementing this bit is optional. It may be tied to either 0 or 1."

I think the behavior is that the bit always "exists"/won't cause an exception. Rather, it can be:

tied to 0

tied to 1

R/W

So, I think the parameter needs to enumerate those possibilities, and based on that, we need a dynamic type()/reset_value(). Since sw_write() doesn't apply to Read-Only bits, and when it is writable there are no restrictions, we don't actually need sw_write().

dhower-qc · 2025-05-28T16:02:33Z

@ThinkOpenly

The regressions are failing because of the various places we've assumed that MXLEN is the fundamental XLEN of the machine. We either need to fix that, or...

I wonder if DXLEN can ever be > than MXLEN? The spec states that DXLEN is:

Debug XLEN, which is the widest XLEN a hart supports, ignoring the current value of mxl in misa.

That seems to be accounting for the fact that misa.MXL used to be mutable. But the spec was amended to make that impossible.

As such, I have trouble imagining any use for a machine where MXLEN == 32 and DXLEN == 64. And it's not worth implementing a DXLEN > MXLEN for the old spec where misa.MXL could change, since the assumption is there are zero implementations out there that do it (it was never well-enough defined anyway).

My vote is to drop DXLEN, and get clarification from the debug spec that DXLEN is always identical to MXLEN.

ThinkOpenly · 2025-05-28T18:02:17Z

My vote is to drop DXLEN, and get clarification from the debug spec that DXLEN is always identical to MXLEN.

Per Paul Donahue (Ventana, author of the Debug spec):

I believe that DXLEN is always the same as MXLEN. I thought that we fixed the definition when mxl became read-only but it looks like that was overlooked.

neverlandiz · 2025-05-28T23:26:19Z

Thanks for the feedback! I'll remove the DXLEN code and use MXLEN for dpc instead.

dhower-qc · 2025-05-29T00:31:35Z

Per Paul Donahue (Ventana, author of the Debug spec):

I believe that DXLEN is always the same as MXLEN. I thought that we fixed the definition when mxl became read-only but it looks like that was overlooked.

Excellent. That's going save us a few headaches.

add dcsr and dpc csrs

38e64d7

neverlandiz requested a review from dhower-qc as a code owner April 14, 2025 18:02

Merge branch 'main' into add-dcsr-and-dpc-csrs

abef190

neverlandiz requested a review from ThinkOpenly as a code owner May 12, 2025 20:27

docs(debug csrs): change dcsr and dpc to D mode

05af687

ThinkOpenly requested changes May 13, 2025

View reviewed changes

neverlandiz added 5 commits May 13, 2025 20:14

docs(debug csrs): add definedBy to fields

a5c6bab

docs(debug csrs): add DXLEN length type

c47a850

docs(debug csrs): fix privilege mode

36de286

docs(debug csrs): add params and sw_write for checking MPRVEN impleme…

fd39d4e

…ntation

docs(debug csrs): fix description

cc34768

ThinkOpenly reviewed May 21, 2025

View reviewed changes

dhower-qc requested changes May 21, 2025

View reviewed changes

neverlandiz added 3 commits May 23, 2025 20:45

docs(debug csrs): fix MPRVEN param

ba6b5d4

docs(debug csrs): add type() and reset_value()

ee80efe

docs(debug csr): add DXLEN to csr.rb

beaeb61

docs(debug csrs): removed DXLEN

d885542

Add DCSR and DPC CSRs #614

Are you sure you want to change the base?

Add DCSR and DPC CSRs #614

Uh oh!

Conversation

neverlandiz commented Apr 14, 2025

Uh oh!

neverlandiz commented Apr 14, 2025

Uh oh!

ThinkOpenly commented Apr 23, 2025

Uh oh!

dhower-qc commented May 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ThinkOpenly May 13, 2025

Choose a reason for hiding this comment

Uh oh!

ThinkOpenly May 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ThinkOpenly May 21, 2025

Choose a reason for hiding this comment

Uh oh!

dhower-qc May 21, 2025

Choose a reason for hiding this comment

Uh oh!

ThinkOpenly commented May 21, 2025

Uh oh!

dhower-qc May 21, 2025

Choose a reason for hiding this comment

Uh oh!

dhower-qc commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThinkOpenly commented May 28, 2025

Uh oh!

neverlandiz commented May 28, 2025

Uh oh!

dhower-qc commented May 29, 2025

Uh oh!

Uh oh!

dhower-qc commented May 28, 2025 •

edited

Loading